Skip to content

SONARJAVA-5160 S1144 Handle arrays in @MethodSource#5638

Merged
tomasz-tylenda-sonarsource merged 2 commits into
masterfrom
tt/method-source-array
May 29, 2026
Merged

SONARJAVA-5160 S1144 Handle arrays in @MethodSource#5638
tomasz-tylenda-sonarsource merged 2 commits into
masterfrom
tt/method-source-array

Conversation

@tomasz-tylenda-sonarsource
Copy link
Copy Markdown
Contributor

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource commented May 28, 2026 •
edited by gitar-bot Bot
Loading

Summary by Gitar

  • Logic enhancements:
    • Updated UnusedPrivateMethodCheck to correctly identify methods referenced as arrays in @MethodSource annotations.
    • Added logic to traverse NEW_ARRAY trees within annotation arguments to extract and filter method names.
  • Test coverage:
    • Added UnusedPrivateMethodSampleTest.java to verify false-positive prevention with array-based @MethodSource configurations.
    • Included a new test case in UnusedPrivateMethodCheckTest to validate the check against the sample file.

This will update automatically on new commits.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 28, 2026 •
edited
Loading

Agentic Analysis: Early Results

Agentic Analysis and Context Augmentation are available on your project. Here are some issues that could have been prevented. Follow the links to learn how to put them into action.

6 issue(s) found across 1 file(s):

Rule File Line Message
java:S1144 java-checks-test-sources/default/src/test/java/checks/tests/UnusedPrivateMethodSampleTest.java 20 Remove this unused private "provideDataFQ" method.
java:S1144 java-checks-test-sources/default/src/test/java/checks/tests/UnusedPrivateMethodSampleTest.java 24 Remove this unused private "provideDataArray1" method.
java:S1144 java-checks-test-sources/default/src/test/java/checks/tests/UnusedPrivateMethodSampleTest.java 28 Remove this unused private "provideDataArray2" method.
java:S1144 java-checks-test-sources/default/src/test/java/checks/tests/UnusedPrivateMethodSampleTest.java 32 Remove this unused private "provideDataArray1Value" method.
java:S1144 java-checks-test-sources/default/src/test/java/checks/tests/UnusedPrivateMethodSampleTest.java 36 Remove this unused private "provideDataArray2Value" method.
java:S1144 java-checks-test-sources/default/src/test/java/checks/tests/UnusedPrivateMethodSampleTest.java 40 Remove this unused private "notUsed" method.

Analyzed by SonarQube Agentic Analysis in 6.3 s

@hashicorp-vault-sonar-prod
Copy link
Copy Markdown
Contributor

hashicorp-vault-sonar-prod Bot commented May 28, 2026 •
edited by atlassian Bot
Loading

SONARJAVA-5160

gitar-bot[bot]
gitar-bot Bot reviewed May 28, 2026
View reviewed changes
Comment thread java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java Outdated
gitar-bot[bot]
gitar-bot Bot reviewed May 28, 2026
View reviewed changes
Comment thread java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java Outdated
gitar-bot[bot]
gitar-bot Bot reviewed May 28, 2026
View reviewed changes
Comment thread java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java Outdated
gitar-bot[bot]
gitar-bot Bot reviewed May 28, 2026
View reviewed changes
Comment thread java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java Outdated
@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource marked this pull request as ready for review May 28, 2026 11:57
rombirli
rombirli reviewed May 29, 2026
View reviewed changes
Comment thread java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java

private void removeMethodNames(NewArrayTree newArrayTree) {
for (ExpressionTree initializer : newArrayTree.initializers()) {
if (initializer.is(Tree.Kind.STRING_LITERAL)) {
Copy link
Copy Markdown
Contributor

@rombirli rombirli May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition should always be true (or the code doesn't compile)

Copy link
Copy Markdown
Contributor Author

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initializer must be known at compile time, but does not have to be a literal. For example, the following is fine:

private static final String PROVIDE_DATA_ARRAY = "provideDataArray";

  @ParameterizedTest
  @MethodSource(value = {PROVIDE_DATA_ARRAY, PROVIDE_DATA_ARRAY + "2"})
  void testAddArray(int num) {}

This is, however, a corner case, and I don't think supporting it is a good use of time.

rombirli
rombirli approved these changes May 29, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@rombirli rombirli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, minor suggestions

@sonarqube-next
Copy link
Copy Markdown

sonarqube-next Bot commented May 29, 2026

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
97.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource merged commit 2439c92 into master May 29, 2026
15 checks passed
@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource deleted the tt/method-source-array branch May 29, 2026 15:03
@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented May 29, 2026

Code Review ✅ Approved 4 resolved / 4 findings

Updates UnusedPrivateMethodCheck to correctly identify array-based @MethodSource references, successfully resolving issues with named array arguments, debug artifacts, and Javadoc errors.

✅ 4 resolved
✅ Quality: Unused method overload: removeMethodName(String)

📄 java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java:296-298
The new removeMethodName(String value) method at line 296-298 is never called anywhere in the file. All call sites pass a LiteralTree argument, invoking the removeMethodName(LiteralTree) overload instead. This dead code should be removed or used.

✅ Quality: Javadoc typo: @MethodsSource should be @MethodSource

📄 java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java:132 📄 java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java:135
The Javadoc comment at line 132 says @MethodsSource (plural) but the actual annotation is @MethodSource. Also the closing tag </> at line 135 should be </li>.

✅ Quality: Debug variables left in production code (lines 117-119)

📄 java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java:117-119
Three variables (unusedMethodsStr, filterStr, methodSourceAnnotatedMethodsStr) are declared at lines 117-119 but never used anywhere in the file. These appear to be leftover debugging aids from development. They duplicate work already done (e.g., methodNames at line 110 is effectively the same as unusedMethodsStr) and will confuse future readers.

Since the PR title says "dirty impl", this is likely intentional for now but should be cleaned up before merge.

✅ Bug: Named array arg @MethodSource(value={...}) not handled

📄 java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java:313-321 📄 java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java:323-327 📄 java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateMethodCheck.java:314-322
The new array handling at line 313 only matches when the annotation argument itself is a NEW_ARRAY (i.e., the implicit value case: @MethodSource({"a", "b"})). However, when the user writes @MethodSource(value = {"a", "b"}), the argument is an AssignmentExpressionTree whose expression is a NEW_ARRAY. The existing assignment branch at line 323 only handles STRING_LITERAL expressions, so this case falls through unhandled, causing a false positive.

The fix should also handle NEW_ARRAY inside an assignment expression.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact 
gitar auto-apply:on         

gitar display:verbose

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gitar-bot gitar-bot[bot] gitar-bot[bot] left review comments

@rombirli rombirli rombirli approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tomasz-tylenda-sonarsource @rombirli